GH-3070: Fix inconsistency between best path and scores in ViterbiDecoder #3189
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @alanakbik @whoisjones, any thought on this topic? I think it should be discussed regarding the consequences of the CRF scoring implementation. |
|
@alanakbik @whoisjones @helpmefindaname it could be great to discuss this topic before the release of flair 0.13. Thanks in advance! |
|
Hi @mauryaland , |
|
Thanks @mauryaland for improving this! Code to test: from flair.data import Sentence
from flair.models import SequenceTagger
# example sentence
sentence = Sentence("I work for the Humboldt Universität of Berlin")
# print NER tags
tagger = SequenceTagger.load("ner-fast")
tagger.predict(sentence, return_probabilities_for_all_classes=True)
print(sentence)
# one token has a higher probability for another class pre-viterbi
difficult_token = sentence[6]
# print the problem token "of"
print(difficult_token)
# print token prediction probabilities
sorted_predictions = sorted(difficult_token.get_tags_proba_dist("ner"), key=lambda x: x.score, reverse=True)
print(sorted_predictions[:2])Before the PR, the token "of" will have the following individual probabilities: ['Token[6]: "of"'/'O' (0.8465999960899353), 'Token[6]: "of"'/'I-ORG' (0.08910000324249268)]After the PR, the token "of" will have the following individual probabilities: ['Token[6]: "of"'/'I-ORG' (0.8465999960899353), 'Token[6]: "of"'/'O' (0.08910000324249268)]Since it is labeled as I-ORG after Viterbi, the score post-PR is more appropriate. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #3070. It is the same fix used back then #949 that has been removed with the refacto. It is definitely not a good interpretation of probabilities from the Viterbi decoding but at least avoid confusion.
Some improvements could be achieved, see for example this solution from Manning stanfordnlp/stanza#744 (comment) and this CRF implementation https://github.com/RasaHQ/rasa/blob/main/rasa/utils/tensorflow/crf.py.
I would be happy to discuss more about what can be done.